Skip to content

Reject individual records when the mock database is over quota - #582

Open
sdvz wants to merge 2 commits into
pointfreeco:mainfrom
sdvz:mock-quota-per-record-rejection
Open

sdvz wants to merge 2 commits into
pointfreeco:mainfrom
sdvz:mock-quota-per-record-rejection

Conversation

@sdvz

@sdvz sdvz commented Nov 11, 2026 •

Copy link
Copy Markdown

The second commit adds a test that fails on main. It reproduces #547.

The test drives MockCloudDatabase with ordinary writes and sends. It does not build a sync engine event by hand.

The limit in the mock

MockCloudDatabase.modifyRecords treated an over-quota batch as all or nothing. It reverted the batch and then failed every record with quotaExceeded.

The real server keeps the records that fit and rejects the other records. A child record next to a rejected parent gets referenceViolation, not quotaExceeded.

The logs on #548 show this pair of errors:

Save failed | parents  | ...:parents  | quotaExceeded (25)
Save failed | children | ...:children | referenceViolation (31)

The mock did not produce that pair. Therefore no test reached the referenceViolation branch of handleSentRecordZoneChanges with a parent that never went to the server.

saveRecordToDatabase already rejects a child when the server does not hold the parent, but that check exempts a parent that is present in recordsToSave. The exemption is correct when the save of the parent succeeds. It is wrong when that save fails, and nothing looked again.

The changes

  • An over-quota batch gives the space that remains to the smallest records. It rejects the largest records with quotaExceeded.
  • The mock then rejects a saved record if the server does not hold its parent. The error is referenceViolation. This step repeats until no record changes, because a rejected child orphans its own children.
  • This step also runs on the normal save path. A parent can fail for other reasons than quotaExceeded.
  • The mock writes saves first and rejects some of them afterwards. A rejection now puts the record back to the version the server held before the batch, so a failed update leaves the previous version in place instead of deleting the record.

The test

listAndReminderRejectedWhileStorageIsFull seeds one list (parent) with a large title and one small reminder (child). Then it sends the batch.

The list fails with quotaExceeded. The reminder fails with referenceViolation. The handler applies the ON DELETE CASCADE on reminders.remindersListID, and deletes a row that the server never held.

Two expectations then fail. Line 502 expects the reminder row on the device. Line 518 expects the reminder to reach the server after the quota rises.

This test reproduces the fault on e72164c, with #335, #571, #576 and #575 merged.

Two questions

  1. The mock's quota is a record count, so a batch that does not fit needs a rule for which records to reject. I order the rejections by encoded record size. That is the smallest change that gives a test a large parent that does not fit next to a small child that would fit. A byte count for quota is closer to real CloudKit. I will change it if you prefer that.
  2. The mock cannot drive the retry path that Always enqueue pending record changes when preparing batches #576 added, so I left that path untested. sendPendingRecordZoneChanges reports an issue and returns unless pendingRecordZoneChanges is not empty, or hasPendingUntrackedChanges is true. Persisted PendingRecordZoneChange rows satisfy neither check, and Always enqueue pending record changes when preparing batches #576 enqueues them later, inside batch preparation. Did I miss a way to drive that path from a test?

@sdvz
sdvz marked this pull request as draft November 11, 2026 18:14
@sdvz
sdvz force-pushed the mock-quota-per-record-rejection branch from e886e2f to 74c1a7c Compare November 11, 2026 18:44
@sdvz sdvz changed the title Reject individual records in the mock's quota emulation Reject individual records when the mock database is over quota Nov 11, 2026
@sdvz
sdvz force-pushed the mock-quota-per-record-rejection branch from 74c1a7c to c5bb729 Compare November 11, 2026 18:58
sdvz added 2 commits November 11, 2026 16:07
The mock treated an over-quota batch as all or nothing. It reverted the
batch and failed every record with 'quotaExceeded'. The real server keeps
the records that fit and rejects the other records. A child next to a
rejected parent gets 'referenceViolation' instead.

The mock now gives the space that remains to the smallest records. It
rejects the largest records with 'quotaExceeded'. It then rejects a
record if the server does not hold the parent of that record. The orphan
pass also runs on the normal save path, because a parent can fail for
other reasons than quota.

'quota' is still a count of records. Size only puts the rejections in
order inside a batch that does not fit.
The test sends a list with a large title and its small reminder in one
batch. The list fails with 'quotaExceeded'. The reminder fails with
'referenceViolation'. The handler for 'referenceViolation' applies
ON DELETE CASCADE to the reminder. It deletes a row that the server
never held.

The test fails on main. See pointfreeco#547.
@sdvz
sdvz force-pushed the mock-quota-per-record-rejection branch from c5bb729 to 726e49b Compare November 11, 2026 23:09
@sdvz
sdvz marked this pull request as ready for review November 11, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant